Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix multi-asic behaviour for queuestat #3554

Merged
merged 1 commit into from
Sep 27, 2024

Conversation

arista-hpandya
Copy link
Contributor

@arista-hpandya arista-hpandya commented Sep 23, 2024

What I did

  • Added support for iterating over all namespaces (ns) when none specified
  • Added a test case to verify all ns behaviour
  • Introduced a wrapper class to handle the mutli-asic functionality
  • Replaced argparse with click for better argument checks

This is in accordance to the issue sonic-net/sonic-buildimage#15148 which tracks the multi-asic support for scripts in sonic-utilities.

How I did it

Modified the queuestat script and associated test files.

How to verify it

The changes were verified by:

  • Running all the unit tests in conic-utilities
  • Verifying the changes manually on a T2 single-asic linecard
  • Verifying the changes manually on a multi-asic linecard

Previous command output (if the output of a command-line utility has changed)

Cmd: queuestat
Result: Empty

Reason: When the namespace (-n/--namespace) argument is not specified on a multi asic device the scripts will return nothing.

New command output (if the output of a command-line utility has changed)

Cmd: queuestat
Result: Runs on all asics present on the device.

Reason: To be consistent with the other PRs in #15148, we iterate over all namespaces when none is specified on a multi-asic device.

Test snapshots provided below:

Specifying invalid namespace on single asic devices: queuestat -n asic0

single asic invalid arg

Single asic behaviour is preserved: queuestat -p Ethernet152

single asic normal

Multi asic behaviour when namespace is specified: queuestat -n asic1 -p Ethernet-Rec1

multiasic specific asic

Multi asic behaviour when no namespace is specified: queuestat

multiasic all ns p1
.
.
.
multiasic all ns p2
.
.
.

- Added support for iterating over all namespaces (ns) when none specified
- Added a test case to verify all ns behaviour
- Introduced a wrapper class to handle the mutli-asic functionality
- Replaced argparse with click for better argument checks
@kenneth-arista
Copy link
Contributor

Copy link
Contributor

@wenyiz2021 wenyiz2021 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

result LGTM, haven't check code details

@arista-hpandya
Copy link
Contributor Author

arista-hpandya commented Sep 25, 2024

@wenyiz2021 Can we add the backport template in the PR description similar to sonic-mgmt? This will help to remind us to mention the branches the change should be cherry-picked in when opening a PR in the future.

Back port request

  • 202012
  • 202205
  • 202305
  • 202311
  • 202405

@vmittal-msft
Copy link
Contributor

LGTM. Did we verify single asic box to make sure it is still good ? If so, we can merge.

@arista-hpandya
Copy link
Contributor Author

LGTM. Did we verify single asic box to make sure it is still good ? If so, we can merge.

Yes, single asic snapshots are attached in the PR description.

@vmittal-msft vmittal-msft merged commit 94ec710 into sonic-net:master Sep 27, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants